Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tests for error cases in did-resolver.spec.ts #561

Merged
merged 5 commits into from
Oct 17, 2023

Conversation

shobitb
Copy link
Contributor

@shobitb shobitb commented Oct 16, 2023

This commit adds new tests in did-resolver.spec.ts to cover some of the previously uncovered error-related branching paths.

This commit contributes partly to issue #136.

Based on the output of npm run test:node before vs. after this commit, the % Branch is now increased by 9.66 percentage points, and the % Stmt and % Lines by 3.29 percentage points.

Before:

did-resolver.ts |   87.91 |    78.57 |   66.66 |   87.91

After:

did-resolver.ts |    91.2 |    88.23 |   66.66 |    91.2 |

This commit adds new tests in did-resolver.spec.ts to cover some of the previously uncovered error-related branching paths.

This commit contributes partly to issue decentralized-identity#136.

Based on the output of `npm run test:node` before vs. after this commit, the `% Branch` is now increased by 9.66 percentage points, and the `% Stmt` and `% Lines` by 3.29 percentage points.

Before:
```
did-resolver.ts |   87.91 |    78.57 |   66.66 |   87.91
```

After:
```
did-resolver.ts |    91.2 |    88.23 |   66.66 |    91.2 |
```
@codesandbox
Copy link

codesandbox bot commented Oct 16, 2023

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

Copy link
Contributor

@thehenrytsai thehenrytsai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shobitb thanks for helping us increase CC! Could you opportunistically convert the Error used in DidResolver into DwnError and introduce new error codes? Search for DwnError and you'll find pattern used in code and tests.

@shobitb
Copy link
Contributor Author

shobitb commented Oct 16, 2023

@shobitb thanks for helping us increase CC! Could you opportunistically convert the Error used in DidResolver into DwnError and introduce new error codes? Search for DwnError and you'll find pattern used in code and tests.

Yes, most def. Let me take a look and will send an updated PR.

This commit adds new tests in did-resolver.spec.ts to cover some of the previously uncovered error-related branching paths.

This commit contributes partly to issue decentralized-identity#136.

Based on the output of `npm run test:node` before vs. after this commit, the `% Branch` is now increased by 9.66 percentage points, and the `% Stmt` and `% Lines` by 3.29 percentage points.

Before:
```
did-resolver.ts |   87.91 |    78.57 |   66.66 |   87.91
```

After:
```
did-resolver.ts |   92.34 |    94.11 |   66.66 |   92.34 |
```
@shobitb
Copy link
Contributor Author

shobitb commented Oct 16, 2023

@thehenrytsai — I've added new error codes and used them in did-resolver, but also changed a few error handling branches in did.ts using the new codes. How does this look?

Thanks!

This commit adds new tests in did-resolver.spec.ts to cover some of the previously uncovered error-related branching paths.

This commit contributes partly to issue decentralized-identity#136.

Based on the output of `npm run test:node` before vs. after this commit, the `% Branch` is now increased by 9.66 percentage points, and the `% Stmt` and `% Lines` by 3.29 percentage points.

Before:
```
did-resolver.ts |   87.91 |    78.57 |   66.66 |   87.91
```

After:
```
did-resolver.ts |   92.34 |    94.11 |   66.66 |   92.34 |
```
src/did/did.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@thehenrytsai thehenrytsai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last nit then good to go!

@shobitb
Copy link
Contributor Author

shobitb commented Oct 16, 2023

Last nit then good to go!

Gotcha! I originally thought the codes were like HTTP codes, where a generic code could be used with different specific error messages.

Added a new code for DidNotString in the latest commit

Copy link
Contributor

@thehenrytsai thehenrytsai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to merge! Thanks again!!

@shobitb
Copy link
Contributor Author

shobitb commented Oct 16, 2023

I don't have write access, so please merge when you can. Thanks a lot, @thehenrytsai!

@thehenrytsai thehenrytsai merged commit 4daab00 into decentralized-identity:main Oct 17, 2023
5 checks passed
@EbonyLouis EbonyLouis added the hacktoberfest-accepted Accepted PRs for the hacking month of October label Oct 19, 2023
diehuxx pushed a commit that referenced this pull request Oct 20, 2023
* main:
  Add codecov coverage job in `integrity-check.yml` (#542)
  Add support for compressed secp256k1 publicKey (#567)
  Add range filter support for dataSize (#568)
  Replace `handle<Xyz>` methods in `Dwn` class with overloaded `process… (#566)
  Add tests for error cases in did-resolver.spec.ts (#561)
  add browser tests to gh actions. (#541)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Accepted PRs for the hacking month of October
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants